Adds IR level PGO Instrumentation options #1992
Conversation
chiragramani
changed the title
| if parsedOptions.hasArgument(.profileGenerate) { | ||
| commandLine.appendFlag("-fprofile-generate") | ||
| } |
There was a problem hiding this comment.
This feels like a bug to me. As I understand, Swift's -profile-generate flag does front-end instrumentation, and is different from Clang's -fprofile-generate.
There was a problem hiding this comment.
this behavior is existing in swift-driver today, and I’m a bit wary of changing it here because there may be tests and downstream flows that rely on it. This PR is primarily adding the new pieces, that said, if you feel it’s worth addressing this legacy behavior in the same PR, please let me know and I can look into it.
|
Just a friendly heads-up that new changes need to be added with this process: |
Yes, I’ve added |
Thank you for the heads-up! I’ll take a look and make sure to follow that process. |
This PR introduces three new instrumentation flags and plumbs them through to IRGen: 1. `-ir-profile-generate` - enable IR-level instrumentation. 2. `-cs-profile-generate` - enable context-sensitive IR-level instrumentation. 3. `-ir-profile-use` - IR-level PGO input profdata file to enable profile-guided optimization (both IRPGO and CSIRPGO) **Context:** https://forums.swift.org/t/ir-level-pgo-instrumentation-in-swift/82123 **Swift-driver PR:** swiftlang/swift-driver#1992 **Tests and validation:** This PR includes ir level verification tests, also checks few edge-cases when `-ir-profile-use` supplied profile is either missing or is an invalid IR profile. However, for argument validation, linking, and generating IR profiles that can later be consumed by -cs-profile-generate, I’ll need corresponding swift-driver changes. Those changes are being tracked in swiftlang/swift-driver#1992
This PR introduces three new instrumentation flags and plumbs them through to IRGen: 1. `-ir-profile-generate` - enable IR-level instrumentation. 2. `-cs-profile-generate` - enable context-sensitive IR-level instrumentation. 3. `-ir-profile-use` - IR-level PGO input profdata file to enable profile-guided optimization (both IRPGO and CSIRPGO) **Context:** https://forums.swift.org/t/ir-level-pgo-instrumentation-in-swift/82123 **Swift-driver PR:** swiftlang/swift-driver#1992 **Tests and validation:** This PR includes ir level verification tests, also checks few edge-cases when `-ir-profile-use` supplied profile is either missing or is an invalid IR profile. However, for argument validation, linking, and generating IR profiles that can later be consumed by -cs-profile-generate, I’ll need corresponding swift-driver changes. Those changes are being tracked in swiftlang/swift-driver#1992
chiragramani
force-pushed
the
instr_options
branch
from
83ce50b to
df3add1
Compare
chiragramani
force-pushed
the
instr_options
branch
from
df3add1 to
4edf9dd
Compare
Hey @artemcm , I have updated the PR post following https://github.com/swiftlang/swift-driver?tab=readme-ov-file#rebuilding-optionsswift. Please let me know if you have any other suggestions. |
This PR introduces three new instrumentation flags and plumbs them through to IRGen: 1. `-ir-profile-generate` - enable IR-level instrumentation. 2. `-cs-profile-generate` - enable context-sensitive IR-level instrumentation. 3. `-ir-profile-use` - IR-level PGO input profdata file to enable profile-guided optimization (both IRPGO and CSIRPGO) **Context:** https://forums.swift.org/t/ir-level-pgo-instrumentation-in-swift/82123 **Swift-driver PR:** swiftlang/swift-driver#1992 **Tests and validation:** This PR includes ir level verification tests, also checks few edge-cases when `-ir-profile-use` supplied profile is either missing or is an invalid IR profile. However, for argument validation, linking, and generating IR profiles that can later be consumed by -cs-profile-generate, I’ll need corresponding swift-driver changes. Those changes are being tracked in swiftlang/swift-driver#1992
|
Hey @artemcm / @cachemeifyoucan , when you get a moment, could you take a look at this PR please? Thanks! |
3 participants
This PR introduces two new instrumentation flags and plumbs them through to IRGen:
-ir-profile-generate- enable IR-level instrumentation.-cs-profile-generate- enable context-sensitive IR-level instrumentation.Context: https://forums.swift.org/t/ir-level-pgo-instrumentation-in-swift/82123
Open Questions (Reviewer Input Requested)
Naming:
Is
-ir-profile-generatea good flag name?-cs-profile-generatepairs nicely with Clang’s-fcs-profile-generate. However,-ir-profile-generatefeels closer to Clang's-fprofile-generatebut-profile-generatealready exists in Swift as a frontend level and not IR level. Should it stay as is, or is there a clearer alternative?PR separation strategy:
I also updated the auto-generated Options.swift file in this PR, I am aware its not supposed to manually updated and I have a Swift-side PR open here: Add IRPGO and CSIRPGO options to Swift swift#84335
.
Should I first land a separate PR in Swift adding only the new options, and then follow up with this PR for the Driver + IRGen functionality? My current assumption is that splitting them would help unblock this PR. Is that correct?